[#551] Dependency patch resolution: Only allow patches from trusted d… #588
Conversation
…rusted dependencies
| @@ -146,6 +146,10 @@ public function activate(Composer $composer, IOInterface $io): void | |||
| 'type' => 'string', | |||
| 'default' => 'patches.json', | |||
| ], | |||
| "allow-dependency-patches" => [ | |||
| 'type' => 'list', | |||
| 'default' => null, | |||
There was a problem hiding this comment.
This should probably default to []
| $ignored_dependencies = $this->plugin->getConfig('ignore-dependency-patches'); | ||
|
|
||
| // First check, if we do allow dependency patches at all. | ||
| if ($allowed_dependencies === []) { |
There was a problem hiding this comment.
Maybe empty($allowed_dependencies) in light of defaulting the value to []?
| if (in_array($p['name'], $ignored_dependencies)) { | ||
| continue; | ||
| $allowed = in_array($p['name'], $allowed_dependencies ?? []); | ||
| $ignored = in_array($p['name'], $ignored_dependencies); |
There was a problem hiding this comment.
Maybe we should simplify this a bit and disallow setting both allowed/ignored dependencies. You can either say "I only want patches from these listed deps" or "I want patches from all deps except these specific ones"
It doesn't make sense to say "I want patches from all deps except these specific ones, but only from these listed deps"
There was a problem hiding this comment.
We have 4 possibilities, if we remain the ignored dependencies possibility:
a) Current behavior: i have no allow-dependency-patches, and no ignored-dependency-patches, i wanted to preserve that all dependent patches will be used. (Case null)
b) Current behavior: I have no allow-dependency-patches, and some ignored-dependency-patches, all dependent patches should be installed except those listed in ignored. (Case null)
c) New behavior: I do not want any dependency patches, so allow-dependency should be a empty list. (Case [] )
d) New behavior: I want do allow some packages, but because of composer-merge i forgot, i have forbitten in another file package x/y to dependent patches. So this will not break existing installations but is really an enhancement without breaking.
If its okay to break current behavior, we could do differently. But i wanted to keep existing behavior, and wanted to keep expected behavior, like an empty list of allowed dependency implicit none is allowed, and if i ignore something, it will be ignored.
…ependencies
Description
Relates to #551.
Added config key as decribed in the issue, added handling for whitelist, while not breaking current behavior if new config key is omitted.
Related tasks